-
Notifications
You must be signed in to change notification settings - Fork 262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Node.js ESM #641
base: alpha
Are you sure you want to change the base?
Support Node.js ESM #641
Conversation
A slightly different approach would be to add a |
@jaydenseric @giuseppeg any progress on this? I've seen many issues from people building npm modules that use styled-jsx, like this one: |
having to include the .js extension in the imports is a breaking change |
@giuseppeg what do you mean. I already said:
The imports documented in the readme right now are invalid for Node.js, they just happen to currently work if you have Webpack etc. setup in your project. Nothing is stopping users from importing the old way; we should document the right way though. |
@jaydenseric sorry I meant that by default the docs should mention |
@jaydenseric sorry I updated the comment, would that work? |
There is a lot to unpack in your comment.
Not true, React components are often rendered in a Node.js environment. All Node.js/npm ecosystem tooling should adopt the final Node.js ESM paradigm. For example, there are ESLint plugins that rightfully report that Regardless, even if that code only ran in a browser there is no harm in specifying file extensions; that is actually more browser friendly not less.
If you are referring to import maps, they don't relate to Node.js ESM support. If you are referring to adding a It is also the recommended way to publish packages that support being imported from ESM and required from CJS whilst avoiding the dual package hazard. Individual parts of the API should be published as CJS available via deep imports, and the ESM index should pull in the same CJS (don't publish the same code in multiple builds/formats; that causes the dual package hazard) allowing it to be accessed via ESM named imports. E.g: https://github.com/jaydenseric/extract-files#function-extractfiles
Sorry I don't understand what you mean by this, how how it relates to anything being discussed. |
I agree I just think that
is not wide spread (not saying that it is a good reason).
I was thinking about https://nodejs.org/api/esm.html#esm_conditional_exports i.e. the alternative solution you proposed. How is that a breaking change? Can we default to the current behavior? I should do some research in order to understand the benefits/tradeoffs and make a call. |
Adding an
Often a package supports the same environments for all the published files (e.g. Note that only recent Node.js versions support blacklisting exports via |
Hi @jaydenseric , we're trying to ship a new version with few breaking changes. I feel this is good to include the ESM support there. styled-jsx plugins loading need to be supported with ESM if this is going to landed. Would you support that one as well in the PR and resolve the conflicts, and targeting to |
@huozhi it's great to see some movement towards proper ESM support, thanks for your recent maintenance work and for giving me the opportunity to see this PR through. This PR was mergeable over a year ago when I originally put careful effort into this contribution, and really needed it. Since then I had to migrate away from |
Fixes #640 .